Skip to content

fix: remove trailing comma in UniqueStringList.ToString#3309

Merged
yhl25 merged 1 commit intonumaproj:mainfrom
syayi:fix/uniq-str-list-tostring-trailing-comma
Mar 13, 2026
Merged

fix: remove trailing comma in UniqueStringList.ToString#3309
yhl25 merged 1 commit intonumaproj:mainfrom
syayi:fix/uniq-str-list-tostring-trailing-comma

Conversation

@syayi
Copy link
Member

@syayi syayi commented Mar 13, 2026

Summary

  • ToString() was appending a comma after every element, producing a trailing comma (e.g. "a,b,c," instead of "a,b,c")
  • Switched to strings.Builder to fix the output and avoid repeated string allocations
  • Added missing test coverage for ToString()

Test plan

  • TestUniqueStringList_ToString covers empty list, single element, and multi-element cases
  • All existing tests in pkg/shared/util continue to pass

@syayi syayi requested review from vigith, whynowy and yhl25 as code owners March 13, 2026 05:59
…t.ToString

ToString was appending a comma after every element, producing a trailing
comma (e.g. "a,b,c," instead of "a,b,c"). Switch to strings.Builder to
fix the output and avoid repeated string allocations.

Signed-off-by: Sri Harsha Yayi <sriharshayayi@gmail.com>
@syayi syayi force-pushed the fix/uniq-str-list-tostring-trailing-comma branch from fdc4770 to baeaf7b Compare March 13, 2026 06:04
Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yhl25 yhl25 enabled auto-merge (squash) March 13, 2026 06:12
@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.11%. Comparing base (a2c8dad) to head (baeaf7b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3309      +/-   ##
==========================================
+ Coverage   81.08%   81.11%   +0.02%     
==========================================
  Files         316      316              
  Lines       72631    72633       +2     
==========================================
+ Hits        58896    58915      +19     
+ Misses      13178    13161      -17     
  Partials      557      557              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yhl25 yhl25 merged commit c7e6a9b into numaproj:main Mar 13, 2026
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants